Skip to content

feat(tui): Session Detail modal - full session drill-down with turn timeline and diff#445

Merged
jaeko44 merged 20 commits intomainfrom
task/cfd631a87666-feat-tui-session-detail-modal-full-session-drill
Mar 30, 2026
Merged

feat(tui): Session Detail modal - full session drill-down with turn timeline and diff#445
jaeko44 merged 20 commits intomainfrom
task/cfd631a87666-feat-tui-session-detail-modal-full-session-drill

Conversation

@jaeko44
Copy link
Copy Markdown
Member

@jaeko44 jaeko44 commented Mar 26, 2026

Task-ID: cfd631a8-7666-458d-9b2b-5bddd4bd9318\n\nAutomated PR for task cfd631a8-7666-458d-9b2b-5bddd4bd9318\n\n---\n\nBosun-Origin: created

@jaeko44 jaeko44 added the bosun-attached Bosun PR attachment marker label Mar 26, 2026
Copilot AI review requested due to automatic review settings March 26, 2026 01:01
@github-actions github-actions bot added the bosun-pr-public PR observed by Bosun but not trusted for high-risk automation label Mar 26, 2026
@jaeko44 jaeko44 added the bosun-needs-fix Attached PR with failing CI that Bosun should pick up for repair label Mar 26, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a richer “Session Detail” modal to the Ink TUI Agents screen, enabling drill-down into a selected session with a turn timeline, recent diff context, and live output/steering controls.

Changes:

  • Implement SessionDetail modal UI with metadata, turn timeline (scrollable), diff preview, and a right-side log panel when wide enough.
  • Add steering input flow and periodic session detail polling while the modal is open.
  • Extend TUI rendering tests and fixtures to cover modal open/close, steering, timeline scrolling, and log streaming.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
tui/screens/agents.mjs Implements the session detail modal, polling, steering input handling, diff/log rendering, and timeline scrolling logic.
tests/tui/screens.test.mjs Adds integration-style Ink render tests for the new modal behaviors and streaming updates.
tests/tui/fixtures.mjs Expands fixtures with turns, metadata fields, and a longer diff payload for modal rendering scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +358 to +366
React.useEffect(() => {
if (!wsBridge || typeof wsBridge.on !== "function") return undefined;
const off = wsBridge.on("logs:stream", (payload) => {
const payloadSessionId = String(payload?.sessionId || payload?.id || payload?.session?.id || "");
if (!detailView?.session?.id || payloadSessionId !== String(detailView.session.id)) return;
const line = streamPayloadToLogLine(payload);
if (!line) return;
setLogLines((current) => [...current.slice(-(MAX_LOG_LINES - 1)), line]);
});
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logs:stream handler filters on payload.sessionId/payload.id, but the canonical TUI logs:stream payload contract does not include any session identifier (it is {logType, query, filePath, line, raw, level, timestamp} with additionalProperties: false). In production this will likely drop all streamed log lines while the detail modal is open. Consider either (a) not filtering by session at all, (b) filtering using payload.query / payload.filePath in a way that matches how logs are subscribed, or (c) extending the event contract + emitter to include a sessionId field and updating the schema accordingly.

Copilot uses AI. Check for mistakes.
@@ -129,6 +312,24 @@ export default function AgentsScreen({ wsBridge, host = "127.0.0.1", port = 3080
});
}, []);

Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The screen no longer has a periodic tick to keep clockMs advancing and to re-run reconcileSessionEntries(...) when the upstream sessions prop is unchanged. This will cause age/idle counters (and any retention logic in reconcileSessionEntries) to freeze until a sessions update arrives. Reintroduce a setInterval (e.g. 1s) that updates clockMs and recomputes entries from liveSessionsRef.current (and make sure it’s cleaned up on unmount).

Suggested change
React.useEffect(() => {
const intervalId = setInterval(() => {
const now = Date.now();
setClockMs(now);
setEntries((previous) => {
const nextEntries = reconcileSessionEntries(previous, liveSessionsRef.current, now);
setSelectedId((current) => {
if (current && nextEntries.some((entry) => entry.id === current)) return current;
return nextEntries[0]?.id || "";
});
return nextEntries;
});
}, 1000);
return () => {
clearInterval(intervalId);
};
}, []);

Copilot uses AI. Check for mistakes.
Comment on lines +223 to +226
sessionId: "session-active-1",
timestamp: "2026-03-23T00:00:31.000Z",
stream: "stdout",
line: "Steer message accepted by running session",
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test emits a logs:stream payload with {sessionId, stream, line}, but the repo’s canonical logs:stream event contract requires {logType, raw, line, level, timestamp, filePath} (and forbids additional properties). The test should use the canonical shape (or, if the intent is to add sessionId to the contract, update the schema + emitter and then align this test with that new contract).

Suggested change
sessionId: "session-active-1",
timestamp: "2026-03-23T00:00:31.000Z",
stream: "stdout",
line: "Steer message accepted by running session",
logType: "stdout",
raw: "Steer message accepted by running session",
line: "Steer message accepted by running session",
level: "info",
timestamp: "2026-03-23T00:00:31.000Z",
filePath: "",

Copilot uses AI. Check for mistakes.
Comment on lines +265 to +266


Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing blank lines at end of file were introduced here; please remove to keep diffs clean.

Suggested change

Copilot uses AI. Check for mistakes.
@jaeko44 jaeko44 added bosun-needs-fix Attached PR with failing CI that Bosun should pick up for repair and removed bosun-needs-fix Attached PR with failing CI that Bosun should pick up for repair labels Mar 26, 2026
@jaeko44 jaeko44 force-pushed the task/cfd631a87666-feat-tui-session-detail-modal-full-session-drill branch from f459012 to db3dfa0 Compare March 26, 2026 02:37
@jaeko44 jaeko44 added bosun-needs-fix Attached PR with failing CI that Bosun should pick up for repair bosun-pr-bosun-created PR created by Bosun and eligible for Bosun automation and removed bosun-needs-fix Attached PR with failing CI that Bosun should pick up for repair bosun-pr-public PR observed by Bosun but not trusted for high-risk automation labels Mar 26, 2026
@github-actions
Copy link
Copy Markdown

Bosun PR classification: Bosun-created.
This PR is tracked by Bosun attachment automation.

  • PR class label: bosun-pr-bosun-created
  • Attach label: bosun-attached (yes, because Bosun-created PRs remain attached regardless of human PR attach mode)
  • Attach mode policy: all
  • Trusted author: no
  • Bosun-created label present: yes
  • Automation scope: Eligible for Bosun repair and merge automation (Bosun-created PR).
  • CI failure signal label: bosun-needs-fix
  • CI failure marker:
  • Trigger: pull_request_target / synchronize

jaeko44 added 2 commits March 26, 2026 23:46
Co-authored-by: bosun-ve[bot] <262908237+bosun-ve[bot]@users.noreply.github.com>
Co-authored-by: bosun-ve[bot] <262908237+bosun-ve[bot]@users.noreply.github.com>
@jaeko44 jaeko44 added bosun-needs-fix Attached PR with failing CI that Bosun should pick up for repair and removed bosun-needs-fix Attached PR with failing CI that Bosun should pick up for repair labels Mar 26, 2026
@jaeko44 jaeko44 added bosun-needs-fix Attached PR with failing CI that Bosun should pick up for repair and removed bosun-needs-fix Attached PR with failing CI that Bosun should pick up for repair labels Mar 27, 2026
@jaeko44
Copy link
Copy Markdown
Member Author

jaeko44 commented Mar 30, 2026

@copilot

🧪 Workflow Sandbox CI / ✅ All Workflow Tests Pass (pull_request)
🧪 Workflow Sandbox CI / ✅ All Workflow Tests Pass (pull_request)Failing after 4s
🧪 Workflow Sandbox CI / 📊 Node-Type Coverage Gate (pull_request)
🧪 Workflow Sandbox CI / 📊 Node-Type Coverage Gate (pull_request)Failing after 22s
🧪 Workflow Sandbox CI / 📋 Template Integrity (pull_request)
🧪 Workflow Sandbox CI / 📋 Template Integrity (pull_request)Failing after 22s
CI / Build + Tests (pull_request)
CI / Build + Tests (pull_request)Failing after 43s
Required

Make sure all tests pass

jaeko44 and others added 6 commits March 31, 2026 07:23
- Resolve merge conflicts in tui/screens/agents.mjs:
  - Keep improved 1s interval that also updates selectedId (reviewer suggestion)
  - Restore refreshData() call on mount from remote
  - Restore full wsBridge handlers (sessions:update, session:event, retry:update)
  - Use simple logs:stream handler without sessionId/workspace filtering
    per reviewer comment (option a: no filtering)

- Fix logs:stream test payload to use canonical shape:
  {logType, raw, line, level, timestamp, filePath}

- Fix SessionDetail layout: remove position=absolute (clips content to
  parent height), use full-screen exclusive modal layout instead

- Fix loadDetail: remove setLogLines(messages) that overwrites streamed
  log lines before they can be rendered

- Fix turn timeline timestamp padding from 19 to 23 chars

- Fix test assertions: wait for async loadDetail completion, use
  observable timestamp substring for log stream test

- Fix streamPayloadToLogLine to include raw in fallback chain

All 10 TUI screen tests pass.

Co-authored-by: jaeko44 <9289791+jaeko44@users.noreply.github.com>
…on-detail-modal-full-session-drill' into task/cfd631a87666-feat-tui-session-detail-modal-full-session-drill

Co-authored-by: jaeko44 <9289791+jaeko44@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 30, 2026

@copilot

🧪 Workflow Sandbox CI / ✅ All Workflow Tests Pass (pull_request)
...

All CI failures and the 4 failing TUI modal tests have been fixed in commit 702a104.

Key fixes:

  • Resolved workflow-templates/github.mjs merge conflict (JS parse error blocking CI)
  • Fixed SessionDetail modal layout — replaced position="absolute" (which clipped content to parent height) with exclusive modal/table rendering
  • Fixed logs:stream handler — removed non-canonical sessionId/workspace filtering per reviewer option (a); listener registered once on mount to avoid timing issues
  • Fixed loadDetail() — removed setLogLines(messages) that overwrote streamed log lines before they could render
  • Fixed logs:stream test payload to canonical shape {logType, raw, line, level, timestamp, filePath}
  • Added 1s interval to keep clockMs, entries, and selectedId current (exact reviewer suggestion)

All 10 TUI screen tests now pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bosun-attached Bosun PR attachment marker bosun-pr-bosun-created PR created by Bosun and eligible for Bosun automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants